Skip to content

Conversation

@sgasho
Copy link
Contributor

@sgasho sgasho commented Dec 14, 2025

related pr: #149271

r? @ZuseZ4

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 14, 2025
@sgasho sgasho changed the title test: update llvm ir expectations for autodiff tests test: update llvm ir expectations for autodiff tests when dlopen Dec 14, 2025
@sgasho sgasho marked this pull request as ready for review December 14, 2025 12:26
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 14, 2025
@sgasho sgasho mentioned this pull request Dec 14, 2025
@sgasho
Copy link
Contributor Author

sgasho commented Dec 16, 2025

  • feat: dlopen Enzyme #149271 has been merged. I'm going to pull --rebase main and recheck to see if all the tests pass
  • I leave this PR as a draft until I confirm that all the tests pass

@sgasho sgasho marked this pull request as draft December 16, 2025 16:16
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2025
@sgasho sgasho force-pushed the test/update_autodiff_related_tests branch from 2cefacd to 5e6d5bb Compare December 17, 2025 15:33
@sgasho
Copy link
Contributor Author

sgasho commented Dec 17, 2025

I confirmed that all tests pass

スクリーンショット 2025-12-18 0 22 27 スクリーンショット 2025-12-18 0 23 20 スクリーンショット 2025-12-18 0 24 07 スクリーンショット 2025-12-18 0 24 43

@sgasho sgasho marked this pull request as ready for review December 17, 2025 15:41
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 17, 2025
@sgasho
Copy link
Contributor Author

sgasho commented Dec 17, 2025

(I don't have much time to wait CI results so I'm re-opening this PR now, during CIs are running)


// d_square2
// CHECK: define internal [4 x float] @fwddiffe4square(ptr noalias noundef readonly align 4 captures(none) dereferenceable(4) %x, [4 x ptr] %"x'")
// CHECK: define internal fastcc [4 x float] @fwddiffe4square(float %x.0.val, [4 x ptr] %"x'")
Copy link
Member

@workingjubilee workingjubilee Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the linkage probably matters to autodiff tests but the calling convention and the first argument maybe shouldn't, since this is optimization sensitive and we are just trying to verify that batching works: use regexes to ignore these details instead. like this, or just {{( fastcc)?}} like you do elsewhere

Suggested change
// CHECK: define internal fastcc [4 x float] @fwddiffe4square(float %x.0.val, [4 x ptr] %"x'")
// CHECK: define internal {{[ a-z_]*}} [4 x float] @fwddiffe4square({{[ a-z0-9()_]*}} %x{{.*}}, [4 x ptr] %"x'")

// d_square3, the extra float is the original return value (x * x)
// CHECK: define internal { float, [4 x float] } @fwddiffe4square.1(ptr noalias noundef readonly align 4 captures(none) dereferenceable(4) %x, [4 x ptr] %"x'")
// d_square1, the extra float is the original return value (x * x)
// CHECK: define internal fastcc { float, [4 x float] } @fwddiffe4square.{{[0-9]+}}(float %x.0.val, [4 x ptr] %"x'")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same idea as my comment on R26

Comment on lines +47 to +48
// CHECK-NEXT: %15 = insertvalue [4 x float] %12, float %14, 3
// CHECK-NEXT: ret [4 x float] %15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one could use a capture here to avoid being so sensitive to specific values

Suggested change
// CHECK-NEXT: %15 = insertvalue [4 x float] %12, float %14, 3
// CHECK-NEXT: ret [4 x float] %15
// CHECK-NEXT: %[[RETVAR:[0-9]+]] = insertvalue [4 x float] %{{[0-9]+}, float %{{[0-9]+}, 3
// CHECK-NEXT: ret [4 x float] %[[RETVAR]]

Comment on lines +36 to +46
// CHECK-NEXT: %4 = fadd fast float %"_2'ipl", %"_2'ipl"
// CHECK-NEXT: %5 = fmul fast float %4, %x.0.val
// CHECK-NEXT: %6 = insertvalue [4 x float] undef, float %5, 0
// CHECK-NEXT: %7 = fadd fast float %"_2'ipl1", %"_2'ipl1"
// CHECK-NEXT: %8 = fmul fast float %7, %x.0.val
// CHECK-NEXT: %9 = insertvalue [4 x float] %6, float %8, 1
// CHECK-NEXT: %10 = fadd fast float %"_2'ipl2", %"_2'ipl2"
// CHECK-NEXT: %11 = fmul fast float %10, %x.0.val
// CHECK-NEXT: %12 = insertvalue [4 x float] %9, float %11, 2
// CHECK-NEXT: %13 = fadd fast float %"_2'ipl3", %"_2'ipl3"
// CHECK-NEXT: %14 = fmul fast float %13, %x.0.val
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless ZuseZ4 prefers this sort of exact matching, this could use CHECK-DAG so we can capture the operations we're actually concerned about: four insertvalues, and an fadd and fmul apiece, in any order.

Comment on lines +62 to +78
// CHECK-NEXT: %_0 = fmul float %x.0.val, %x.0.val
// CHECK-NEXT: %4 = fadd fast float %"_2'ipl", %"_2'ipl"
// CHECK-NEXT: %5 = fmul fast float %4, %x.0.val
// CHECK-NEXT: %6 = insertvalue [4 x float] undef, float %5, 0
// CHECK-NEXT: %7 = fadd fast float %"_2'ipl1", %"_2'ipl1"
// CHECK-NEXT: %8 = fmul fast float %7, %x.0.val
// CHECK-NEXT: %9 = insertvalue [4 x float] %6, float %8, 1
// CHECK-NEXT: %10 = fadd fast float %"_2'ipl2", %"_2'ipl2"
// CHECK-NEXT: %11 = fmul fast float %10, %x.0.val
// CHECK-NEXT: %12 = insertvalue [4 x float] %9, float %11, 2
// CHECK-NEXT: %13 = fadd fast float %"_2'ipl3", %"_2'ipl3"
// CHECK-NEXT: %14 = fmul fast float %13, %x.0.val
// CHECK-NEXT: %15 = insertvalue [4 x float] %12, float %14, 3
// CHECK-NEXT: %16 = insertvalue { float, [4 x float] } undef, float %_0, 0
// CHECK-NEXT: %17 = insertvalue { float, [4 x float] } %16, [4 x float] %15, 1
// CHECK-NEXT: ret { float, [4 x float] } %17
// CHECK-NEXT: }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all I would say here is reiterating previous commentary

// debug-NEXT: define internal { float, float }
// debug-SAME: (ptr %f, float %x, float %dret)
// release-NEXT: define internal fastcc float
// release-NEXT: define internal fastcc{{( noundef)?}} float
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw autodiff only supports main, no older versions. So I'd not mark it as optional, as I hope that we don't regress. Or, at least I'd like to learn about regressions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.

Suggested change
// release-NEXT: define internal fastcc{{( noundef)?}} float
// release-NEXT: define internal fastcc noundef float

// debug-NEXT: define internal { float, float }
// debug-SAME: (ptr align 4 %x, ptr align 4 %bx_0, ptr align 4 %y, ptr align 4 %by_0)
// release-NEXT: define internal fastcc { float, float }
// release-NEXT: define internal fastcc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is its return type now?

Comment on lines +25 to +27
// CHECK: define internal{{.*}} @diffe_{{.*}}generic6square
// CHECK-SAME: double
// CHECK: f{{add|mul}} {{.*}} double
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm. this looks very different, now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants